Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Archive of our own Provider #173

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

PrecociouslyDigital
Copy link

No description provided.

Copy link
Owner

@LagradOst LagradOst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit unsure if unfinished as not everything is used here, it seams to work at least

@PrecociouslyDigital
Copy link
Author

It's done and ready for merge on my end!

@LagradOst
Copy link
Owner

LagradOst commented Jun 19, 2023

dont do too much UI as all UI is currently being refacored

@PrecociouslyDigital
Copy link
Author

dont do too much UI as all UI is currently being refacored

What part of the UI are you referring to? the login system?

@LagradOst
Copy link
Owner

dont do too much UI as all UI is currently being refacored

What part of the UI are you referring to? the login system?

yup

@PrecociouslyDigital
Copy link
Author

Unfortunately it needs some UI so the user can enter their username and password to access protected works on AO3; The total extent of this is just a setting in the settings menu.

@LagradOst
Copy link
Owner

Unfortunately it needs some UI so the user can enter their username and password to access protected works on AO3; The total extent of this is just a setting in the settings menu.

yes, just dont add anything extra

@PrecociouslyDigital
Copy link
Author

It's ready for review/merge now

@LagradOst LagradOst requested a review from Blatzar June 24, 2023 22:00
@LagradOst
Copy link
Owner

An app wide cookie jar makes everything harder to debug and handle. Easier for us to be explicit with cookies for deterministic behavior.

If you want sessions with cookies I think https://github.com/Blatzar/NiceHttp/blob/master/library/src/main/java/com/lagradost/nicehttp/Session.kt would work good enough for limited time cookies.

At the very least please create your own cookie requests instance instead of an app wide change.

@LagradOst
Copy link
Owner

do note that I removed the cookejar from mainactivity

@PrecociouslyDigital
Copy link
Author

ready for review!

@KillerDogeEmpire
Copy link
Contributor

@PrecociouslyDigital I tried the ao3 provider and tried to log in using the new provider settings, and it just crashed. also the search timed out.

@LagradOst
Copy link
Owner

  1. DONT USE A FRAGMENT. use an alert
  2. Your current code is crashing
  3. Archive of our own Provider is reporting ratings higher than possible in qn you are confusing the variables, read or just do peopleVoted = peopleVoted views = views ect
  4. You use MainActivity.mainActivity everywhere, use context instead as it is a global weak reference or even activity if activity is needed

@PrecociouslyDigital
Copy link
Author

@LagradOst I'd like to clarify; I shouldn't make provider settings a menu of its own?

@LagradOst
Copy link
Owner

@LagradOst I'd like to clarify; I shouldn't make provider settings a menu of its own?

shit forgor to reply, but I want to show an Alert for settings, it is a sort of popup so you dont take up the entire page. Here is an example in cs3. This also makes it possible to show anywhere
image

@PrecociouslyDigital
Copy link
Author

Gotit; I unfortunately will be busy until August so if you or anyone else wants to make these changes I'd be delighted. If not I'll try to get this in by Aug 15th

@LagradOst
Copy link
Owner

Understandable, have a great day

@KillerDogeEmpire
Copy link
Contributor

@PrecociouslyDigital this still in progresss, if not I'll try my best to finish it. I was going to add it ages ago since I wanted to add this for my crush

@PrecociouslyDigital
Copy link
Author

PrecociouslyDigital commented Dec 7, 2023 via email

@cheesy3097
Copy link

cheesy3097 commented Apr 27, 2024

please add local source @LagradOst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants